Ray Serve readiness checks only proxy#4480
Ray Serve readiness checks only proxy#4480spencer-p wants to merge 3 commits intoray-project:masterfrom
Conversation
Signed-off-by: Spencer Peterson <spencerjp@google.com>
spencer-p
requested review from
MortalHappiness,
andrewsykim,
kevin85421 and
rueian
as code owners
|
I want to ask about since the slim images (without wget) are only being released for Ray 2.53+, does older versions need to keep using wget? |
Co-authored-by: Jun-Hao Wan <ken89@kimo.com> Signed-off-by: Spencer Peterson <spencerjp@google.com>
Older versions need to use the exec probe for cases where there's multiple endpoints, which is the case for the all the liveness probes and all the non-serving readiness probes. I suppose if we agree the Serve readiness check only needs to be the proxy endpoint, then we should be able to drop wget for this probe only on pre-2.53 versions. |
| if httpHealthCheck { | ||
| rayContainer.ReadinessProbe.Exec = nil | ||
| rayContainer.ReadinessProbe.HTTPGet = &corev1.HTTPGetAction{ | ||
| Path: utils.RayServeProxyHealthPath, |
There was a problem hiding this comment.
I think this makes, but just calling out the assumption that the server proxy health check likely depends on the raylet health already.
@rueian @Future-Outlier does it make sense to you?
There was a problem hiding this comment.
Will there ever be a case where serve proxy health check passes but raylet probe fails? Seems unlikely to me but maybe I'm missing something
Signed-off-by: Spencer Peterson <spencerjp@google.com>
4 participants
Why are these changes needed?
The RayService readiness check uses wget to fetch both the raylet health and the proxy actor status. It's unclear if both are needed. If we can check only the raylet for liveness, and only the proxy for readiness, then we can completely remove dependencies on wget on 2.53 and later.
Related issue number
Follow up to #4448.
Checks
- I've made sure the tests are passing.
- Testing Strategy
- Unit tests
- Manual tests
- This PR is not tested :(
Cursor Bugbot reviewed your changes and found no issues for commit 96d27be